Skip to content

Conversation

@AkshitaB
Copy link
Contributor

@AkshitaB AkshitaB commented Apr 15, 2022

Right now, steps cannot take Model object as inputs, unless _to_params() is implemented. As it's unreasonable to expect users to write _to_params() for every implementation, we can add any empty implementation of it in the base class. However, that may lead to incorrect hashing for the object potentially?

Note: the actual layers in the model don't matter.

Error observed:

_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
../../tango/common/testing.py:87: in run
    run_name = _run(
../../tango/__main__.py:707: in _run
    run = workspace.register_run((step for step in step_graph.values()), name)
../../tango/workspaces/local_workspace.py:345: in register_run
    all_steps = set(targets)
../../tango/step.py:452: in __hash__
    return hash(self.unique_id)
../../tango/step.py:431: in unique_id
    self.unique_id_cache += det_hash(
../../tango/common/det_hash.py:128: in det_hash
    pickler.dump(o)
../../../../../virtuals/tango/lib/python3.8/site-packages/dill/_dill.py:498: in dump
    StockPickler.dump(self, obj)
/Library/Frameworks/Python.framework/Versions/3.8/lib/python3.8/pickle.py:485: in dump
    self.save(obj)
../../tango/common/det_hash.py:102: in save
    super().save(obj, save_persistent_id)
/Library/Frameworks/Python.framework/Versions/3.8/lib/python3.8/pickle.py:558: in save
    f(self, obj)  # Call unbound method with explicit self
/Library/Frameworks/Python.framework/Versions/3.8/lib/python3.8/pickle.py:884: in save_tuple
    save(element)
../../tango/common/det_hash.py:102: in save
    super().save(obj, save_persistent_id)
/Library/Frameworks/Python.framework/Versions/3.8/lib/python3.8/pickle.py:558: in save
    f(self, obj)  # Call unbound method with explicit self
../../../../../virtuals/tango/lib/python3.8/site-packages/dill/_dill.py:990: in save_module_dict
    StockPickler.save_dict(pickler, obj)
/Library/Frameworks/Python.framework/Versions/3.8/lib/python3.8/pickle.py:969: in save_dict
    self._batch_setitems(obj.items())
/Library/Frameworks/Python.framework/Versions/3.8/lib/python3.8/pickle.py:1000: in _batch_setitems
    save(v)
../../tango/common/det_hash.py:102: in save
    super().save(obj, save_persistent_id)
/Library/Frameworks/Python.framework/Versions/3.8/lib/python3.8/pickle.py:537: in save
    pid = self.persistent_id(obj)
../../tango/common/det_hash.py:107: in persistent_id
    det_hash_object = obj.det_hash_object()
../../tango/common/from_params.py:838: in det_hash_object
    r = (self.__class__.__qualname__, self.to_params())
../../tango/common/from_params.py:820: in to_params
    return Params(replace_object_with_params(self._to_params()))
../../tango/common/from_params.py:809: in replace_object_with_params
    return {key: replace_object_with_params(value) for key, value in o.items()}
../../tango/common/from_params.py:809: in <dictcomp>
    return {key: replace_object_with_params(value) for key, value in o.items()}
../../tango/common/from_params.py:809: in replace_object_with_params
    return {key: replace_object_with_params(value) for key, value in o.items()}
../../tango/common/from_params.py:809: in <dictcomp>
    return {key: replace_object_with_params(value) for key, value in o.items()}
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

o = Sequential(
  (0): FeedForward(
    (linear): Linear(in_features=4, out_features=4, bias=True)
    (activation): ReLU(...)
  )
  (2): FeedForward(
    (linear): Linear(in_features=4, out_features=4, bias=True)
    (activation): ReLU()
  )
)

    def replace_object_with_params(o: Any) -> Any:
        if isinstance(o, FromParams):
            return o.to_params().as_dict(quiet=True)
        elif isinstance(o, (list, tuple, set)):
            return [replace_object_with_params(i) for i in o]
        elif isinstance(o, dict):
            return {key: replace_object_with_params(value) for key, value in o.items()}
        elif isinstance(o, Path):
            return str(o)
        elif o is None or isinstance(o, (str, float, int, bool)):
            return o
        else:
>           raise NotImplementedError(
                f"Unexpected type encountered in to_params(): {o} ({type(o)})\n"
                "You may need to implement a custom '_to_params()'."
            )
E           NotImplementedError: Unexpected type encountered in to_params(): Sequential(
E             (0): FeedForward(
E               (linear): Linear(in_features=4, out_features=4, bias=True)
E               (activation): ReLU()
E             )
E             (1): FeedForward(
E               (linear): Linear(in_features=4, out_features=4, bias=True)
E               (activation): ReLU()
E             )
E             (2): FeedForward(
E               (linear): Linear(in_features=4, out_features=4, bias=True)
E               (activation): ReLU()
E             )
E           ) (<class 'torch.nn.modules.container.Sequential'>)
E           You may need to implement a custom '_to_params()'.

../../tango/common/from_params.py:815: NotImplementedError

@AkshitaB AkshitaB mentioned this pull request Apr 15, 2022
5 tasks
@AkshitaB AkshitaB requested a review from epwalsh April 19, 2022 21:06
@dirkgr dirkgr self-assigned this Apr 28, 2022
@dirkgr
Copy link
Member

dirkgr commented Apr 28, 2022

Note to self: Can we get rid of to_params completely?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants